Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BREAKING CHANGE: (#31) Removing the special nodes classes #33

Merged
merged 13 commits into from
Sep 22, 2024

Conversation

ca11mejane
Copy link
Collaborator

@ca11mejane ca11mejane commented May 12, 2024

Related issue:
resolves #31

Changes:

  • Removed classes: [DataSource, FeatureBase, MLModelBase, FeatureVectorizerBase, EventManagerBase,]
  • Removed types: [NodeLike, ProcessorLike, DataSourceLike, FeatureLike, FeatureVectorizerLike, MLModelLike, NodeProtocol,]
  • Removed attributes: [NodeBase.title,]
  • Added type annotation for: PipelineChart(..., PipelineChartLike) and DAGPipelineContext(..., PipelineContextLike):

@ca11mejane ca11mejane changed the title BREAKING CHANGEL: (#31) Removing the special nodes classes BREAKING CHANGE: (#31) Removing the special nodes classes May 12, 2024
@ca11mejane ca11mejane marked this pull request as draft May 24, 2024 07:10
@ca11mejane
Copy link
Collaborator Author

ca11mejane commented May 24, 2024

marked as WIP untill merge of #35

upd: PR updated with the latest master; marked as ready

@ca11mejane ca11mejane marked this pull request as ready for review June 11, 2024 07:35
'vectorize',
'predict',
)
RUN_METHOD_ALIAS = 'process'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still have a question for this method. If we remove the ability to use different names in subclasses, I guess that we actually don't need to remain logic with a method's name as a string. We can just leave the abstract method "process" and use it. It will help understand codebase much faster and less painfully because the method is more explicit than the string name of the method

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I've removed the constant, but I decided not to touch any other method dependencies; I suggest we take a look and decide what to eliminate from annotations in the next iteration

diff: edd533b

@ca11mejane ca11mejane merged commit 3e85cf0 into master Sep 22, 2024
5 checks passed
@ca11mejane ca11mejane deleted the breaking_change/remove_nodelike branch September 22, 2024 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove exessive structures
3 participants